Add Kilo SDK-backed provider adapter#3882
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Effect service review: one convention issue found in the new Kilo text-generation service. See inline comment.
Posted via Macroscope — Effect Service Conventions
The Kilo SDK injects a spinner + "Initializing snapshot…" text part on the assistant message while it initializes its undo/redo file snapshot, which surfaced as repeated noise in the chat scrollback. Filter it out by content in the adapter so it never reaches the ingestion layer or the WebSocket client, and handle the SDK's new message.part.removed event so per-part state does not leak across turns.
There was a problem hiding this comment.
Effect service conventions review: one finding in the new Kilo text-generation service. Other new Kilo modules (kiloRuntime.ts, KiloAdapter.ts, KiloProvider.ts, KiloDriver.ts) faithfully mirror the established OpenCode service/runtime patterns and look consistent with the repo conventions.
Posted via Macroscope — Effect Service Conventions
…Error detail Replace the Effect.exit/Cause.squash wrap in KiloTextGeneration.ts:91 with a per-tag catchTags so KiloRuntimeError, KiloTextGenerationSessionPayloadError, and KiloTextGenerationEmptyOutputError each translate into a TextGenerationError whose detail is derived from stable, structural attributes (operation name, failure kind) instead of copying cause.message into the wrapper. Cover the two new mapping branches with focused unit tests in KiloTextGeneration.test.ts that exercise the failure paths against an in-memory KiloRuntime test double and assert both the structural detail and that credential/cause text never leaks into the wrapper.
ApprovabilityVerdict: Needs human review 2 blocking correctness issues found. This PR introduces a substantial new provider integration (Kilo SDK) with ~3000 lines of new infrastructure, session management, and event handling. Additionally, unresolved review comments flag high/medium severity bugs in the streaming/delta logic that could corrupt user-visible output. You can customize Macroscope's approvability policy. Learn more. |
…Session mutex, branch sanitization
- packages/contracts/src/model.ts: drop the bogus kilo/ prefix on the Kilo
default model slugs in DEFAULT_MODEL_BY_PROVIDER and
DEFAULT_GIT_TEXT_GENERATION_MODEL_BY_PROVIDER. parseKiloModelSlug expects
two-segment provider/model, so the extra segment was being sent verbatim
to the SDK and breaking inventory lookup.
- apps/server/src/provider/Layers/KiloAdapter.ts:
- sendTurn: when a fresh turn's session.promptAsync rejects, emit a
turn.completed{state:"failed"} alongside the existing rollback so
downstream turn tracking doesn't hang on a dropped request.
- startSession: serialize concurrent calls for the same threadId
through a per-thread Semaphore so two overlapping startSession calls
can't orphan their scope/server/SDK session.
- apps/server/src/textGeneration/KiloTextGeneration.ts: drop the bespoke
sanitizeKiloBranchName in favour of the shared
sanitizeBranchFragment from @t3tools/shared/git, matching the other
providers so model output with illegal git ref characters is rejected
before reaching worktree/branch creation.
Bugbot re-review flagged two issues with the prior per-thread lock: - Racy get-then-set on the lock map could give two concurrent startSession calls each their own Semaphore, defeating mutual exclusion. - stopSession ran unlocked, so a stop could tear down a session the starter was about to register as successful. Adopt the established Cursor/Grok pattern: hold the per-thread semaphores in a SynchronizedRef so get-or-create is atomic, and wrap both startSession and stopSession in withThreadLock so they cannot interleave on the same threadId. stopSession also drops its pre-throw path and just no-ops when the thread has no context, instead of throwing ProviderAdapterSessionNotFoundError, since that error is no longer reachable now that we hold the same mutex as start.
- Replace the synchronous ensureContext throw in interruptTurn, respondToRequest, and respondToUserInput with a typed lookupContext Effect that yields a ProviderAdapterSessionNotFoundError on an unknown thread. Callers outside the Effect runtime no longer see an untyped JavaScript throw, and the error flows through the typed channel like in the other adapters. - When message.updated arrives for an assistant message whose role was unknown at the time any text/reasoning message.part.updated was recorded, backfill those parts through emitText so the assistant text is not silently dropped from the runtime event stream. Add a focused KiloAdapter test for the order-update-before-role scenario.
Bugbot flagged that emitText would silently garble or duplicate assistant text when backfilling parts whose message role was learned late: if a later message.part.updated snapshot was shorter than what had already been streamed via deltas (or via an earlier incomplete snapshot), the previous delta logic fell back to 'delta = text' (full replacement), re-emitting the older content over the already-streamed output. emitText now only emits a content.delta when: - the snapshot is a strict extension of the accumulated stream, or - nothing has been streamed yet for this part id. If the snapshot is shorter than (or otherwise non-prefix-extending) relative to , the snapshot is dropped and the accumulator is left alone. The completion event still fires, so a later-arriving snapshot that carries time.end can still flip the part into the completed state. Add a regression test that exercises the late-role + early-snapshot sequence and verifies the duplicate prefix is not emitted.
| raw: unknown, | ||
| ) { | ||
| const text = textFromPart(part); | ||
| if (text === undefined) return; |
There was a problem hiding this comment.
🟡 Medium Layers/KiloAdapter.ts:261
emitText records the accumulated text into emittedTextByPartId before the snapshot-progress suppression check in the message.part.delta handler, but in emitText itself the accumulator is only written inside the delta-computation branches — which are all skipped when isKiloSnapshotProgressText(text) returns early at line 270. So the comment on line 266 ("The accumulator above is intentionally written before this check") is wrong for the emitText path: nothing is written. If a suppressed message.part.updated is followed by a message.part.delta for the same part, the delta handler reads previous as "" instead of the already-streamed progress text, corrupting the accumulated baseline and emitting synthetic spinner content to the UI. Move the emittedTextByPartId.set(part.id, text) call above the isKiloSnapshotProgressText guard so the accumulator is always recorded regardless of suppression.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/Layers/KiloAdapter.ts around line 261:
`emitText` records the accumulated text into `emittedTextByPartId` *before* the snapshot-progress suppression check in the `message.part.delta` handler, but in `emitText` itself the accumulator is only written inside the delta-computation branches — which are all skipped when `isKiloSnapshotProgressText(text)` returns early at line 270. So the comment on line 266 ("The accumulator above is intentionally written before this check") is wrong for the `emitText` path: nothing is written. If a suppressed `message.part.updated` is followed by a `message.part.delta` for the same part, the delta handler reads `previous` as `""` instead of the already-streamed progress text, corrupting the accumulated baseline and emitting synthetic spinner content to the UI. Move the `emittedTextByPartId.set(part.id, text)` call above the `isKiloSnapshotProgressText` guard so the accumulator is always recorded regardless of suppression.
The accumulator state was being updated even when emitText and the message.part.delta handler suppressed synthetic snapshot-progress text, so a suppressed progress frame could leave a phantom baseline that caused the staleness guard to drop non-suppressed content that arrived later. Likewise, recording suppressed progress text in the delta handler could chain into a non-progress accumulated result that the filter no longer matched, leaking spinner content toward the UI when the next delta extended an unseen progress frame. emitText and message.part.delta now only update emittedTextByPartId when something is actually emitted to the user. emitText's delta selection becomes: - text shorter than : stale snapshot — drop. - text extends : emit the new tail. - otherwise: emit full text (replacement or initial). This keeps the accumulator aligned with what the chat UI actually sees, so subsequent edits compute correct deltas and backfilled message.updated events cannot garble or duplicate the already-streamed output.
| if (text.length < previous.length) { | ||
| delta = ""; | ||
| } else if (text.startsWith(previous) && text.length > previous.length) { | ||
| delta = text.slice(previous.length); | ||
| } else { | ||
| delta = text; | ||
| } |
There was a problem hiding this comment.
🟡 Medium Layers/KiloAdapter.ts:283
When emitText receives a message.part.updated with the same text as the previously emitted value (text === previous), it emits the entire text again as a content.delta. The extension check requires text.length > previous.length, so equal-length text falls through to the replacement branch and re-sends the full string. Repeated identical snapshots therefore duplicate the assistant message in consumers that append deltas. The equality case should produce no delta.
| if (text.length < previous.length) { | |
| delta = ""; | |
| } else if (text.startsWith(previous) && text.length > previous.length) { | |
| delta = text.slice(previous.length); | |
| } else { | |
| delta = text; | |
| } | |
| let delta = ""; | |
| if (text === previous || text.length < previous.length) { | |
| delta = ""; | |
| } else if (text.startsWith(previous) && text.length > previous.length) { | |
| delta = text.slice(previous.length); | |
| } else { | |
| delta = text; | |
| } |
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/Layers/KiloAdapter.ts around lines 283-289:
When `emitText` receives a `message.part.updated` with the same text as the previously emitted value (`text === previous`), it emits the entire text again as a `content.delta`. The extension check requires `text.length > previous.length`, so equal-length text falls through to the replacement branch and re-sends the full string. Repeated identical snapshots therefore duplicate the assistant message in consumers that append deltas. The equality case should produce no delta.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit b4d3f1c. Configure here.
| } else { | ||
| delta = text; | ||
| } | ||
| context.emittedTextByPartId.set(part.id, text); |
There was a problem hiding this comment.
Stale text deltas garbled
High Severity
The emitText function now updates emittedTextByPartId with the current part.text snapshot, even when that snapshot is shorter than the previously emitted text or when no delta was generated. This causes the accumulator to misrepresent the user-visible stream, leading to subsequent deltas garbling or duplicating already-streamed text. Additionally, equal-length repeat snapshots are now emitted as full content.deltas, duplicating the entire message.
Reviewed by Cursor Bugbot for commit b4d3f1c. Configure here.


What Changed
kiloas a first-party provider backed by@kilocode/sdkand a managedkilo serveprocess./providerand stores selections asproviderID/modelIDwhile retaining one top-level Kilo provider.Why
Closes #1579.
Kilo exposes typed server, session, provider, and event APIs through
@kilocode/sdk. Using that integration surface avoids scraping CLI output and allows T3 Code to consume Kilo's live model inventory and structured session stream without exposing Kilo's internal upstream providers or agent modes as separate T3 concepts.The initial adapter intentionally uses Kilo's built-in
codeagent by default, maps plan turns toplan, and treats recovery as best effort.UI Changes
Kilo now appears in provider settings and the model picker. The UI reuses the existing generic provider settings and picker components; no new interaction surface or animation was added.
Video proof
Schermopname.2026-07-11.om.17.06.50.mov
Checklist
Verification
vp checkvp run typecheckvp test run src/provider/kiloRuntime.test.ts src/provider/Layers/KiloProvider.test.ts src/provider/Layers/ProviderInstanceRegistryLive.test.ts(10 tests)kilo serveand loaded/providersuccessfully.Note
Add Kilo SDK-backed provider adapter with full session, streaming, and text generation support
kiloprovider driver (KiloDriver.ts) that manages provider instance lifecycle, periodic status checks, and wires together the adapter and text generation services.makeKiloAdapter(KiloAdapter.ts), a fullProviderAdapterimplementation that manages Kilo CLI sessions, streams assistant content deltas, handles permission/question prompts, supports turn interruption, and exposes thread read/rollback.makeKiloTextGeneration(KiloTextGeneration.ts) for generating commit messages, PR content, branch names, and thread titles via the Kilo SDK.KiloRuntime(kiloRuntime.ts) to spawn and supervise the Kilo CLI process, discover its server URL, and load provider inventory.kiloas a built-in driver in contracts, server settings, and the settings UI, including default models, icon, and an "Early Access" badge.KiloRuntimeLiveis now merged into the application runtime layer alongsideOpenCodeRuntimeLive, which affects all downstream service wiring.Macroscope summarized b4d3f1c.
Note
Medium Risk
New subprocess and SDK integration on the hot path for sessions and streaming; permission rules and event-ordering logic are non-trivial, though heavily tested and failures degrade to provider error snapshots rather than blocking server startup.
Overview
Adds Kilo as a first-class coding provider via
@kilocode/sdk, wired through the same driver/adapter/registry pattern as OpenCode and other built-ins.The server gains a Kilo runtime that spawns
kilo serve, talks to the SDK client, probes CLI version, and loads connected upstream models asproviderID/modelIDslugs. KiloDriver builds managed snapshots (status refresh, manual@kilocode/climaintenance) and bundles the adapter plus KiloTextGeneration for branch names, commit/PR copy, and thread titles.KiloAdapter maps SDK session/event streams into
ProviderRuntimeEvents: turns, assistant/reasoning deltas, tools, permissions, questions, todos, interrupts, rollback, and attachments. It filters synthetic “Initializing snapshot…” spinner text so it never hits the chat UI, and handles out-of-order SDK events (backfill, stale snapshot guards).Contracts add
KiloSettings, server settings/patches, default models, andkilo.sdk.eventraw source. The web app registers Kilo in settings, the model picker (Early Access), and icons. KiloRuntimeLive is merged into server runtime dependencies alongside OpenCode.Reviewed by Cursor Bugbot for commit b4d3f1c. Bugbot is set up for automated code reviews on this repo. Configure here.